Add type hints, fix mypy issues (#198) #228
Conversation
* Specify how to install Django 3.2. These tests are currently running against later Django versions. * Add basepython entries for pypy. This fixes running tox locally. * Fix the cpython version mapping in [gh-actions]. The github action tests for cpython versions are running against the latest Django, instead of the set of possible Django versions.
There are a few test failures with pypy and Django 3.2. Since 3.2 is out of long-term support, drop these from the test matrix.
mypy complains when reading or setting a attribute that is not defined on the class, such as HttpRequest.csp_nonce. This updates the code to use getattr and setattr to access these dynamically added attributes and for Django settings.
Both startswith() and parser.parse_statements take a tuple rather than a list.
Althought the code `template.render(context)` looked similar, mypy complained that Django's Template could not take a dict. Rather than switch on types, refactor `make_context` and `make_template` into `render`, which hides the typing details between Django templates and extension templates like Jinja2.
| @@ -15,12 +17,12 @@ | |||
| class Nonce: | |||
| _instance = None | |||
|
|
|||
| def __new__(cls, *args, **kwargs): | |||
| def __new__(cls: Type["Nonce"], *args: Any, **kwargs: Any) -> "Nonce": | |||
There was a problem hiding this comment.
the way to write this is:
def __new__(cls, *args: Any, **kwargs: Any) -> Self:(Self is importable from typing_extensions for the pythons supported here)
There was a problem hiding this comment.
Yes, this seems to work. Do you want to submit the PR?
There was a problem hiding this comment.
Thanks @asottile-sentry. Do you want to submit a PR?
the py.typed file isn't included in the packaging I think? so these types won't be available to users of the package
I was worried about this too, but I think no further config is needed. When I follow the deploy steps in the GitHub action:
pip install build
python -m build -v .
I see the py.typed file getting added to the package data:
...
* Building sdist...
...
copying csp/models.py -> django_csp-4.0b1/csp
copying csp/py.typed -> django_csp-4.0b1/csp
copying csp/utils.py -> django_csp-4.0b1/csp
...
* Building wheel...
...
copying build/lib/csp/py.typed -> build/bdist.macosx-14.4-arm64/wheel/csp
...
adding 'csp/py.typed'
...
| @@ -15,12 +17,12 @@ | |||
| class Nonce: | |||
| _instance = None | |||
|
|
|||
| def __new__(cls, *args, **kwargs): | |||
| def __new__(cls: Type["Nonce"], *args: Any, **kwargs: Any) -> "Nonce": | |||
There was a problem hiding this comment.
Yes, this seems to work. Do you want to submit the PR?
the automatic inclusion of |
I think we're good then, https://github.com/mozilla/django-csp/actions/runs/9589504359/job/26443444411#step:6:14 |
|
it relies on an experimental feature and it breaks in environments which don't have setuptools 69+ available -- such as building from source either the |
This add
mypyto the test matrix, and type hints to all the code, fixing #198.mypyrequired some code changes, and I've rebased them out into separate commits, which could be added before the big one that hints all the functions.I've built on top of PR #227, which fixes configuration issues with
tox.ini. In that PR, I disabledpypytesting against Django 3.2, which was consistently breaking. I thought these changes fixed that, but they still fail 1 out of 10 times locally, so I'm keeping them disabled.To run
mypylocally: